Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Systemd cleanup in distro #153

Merged
merged 2 commits into from
Dec 9, 2018
Merged

Systemd cleanup in distro #153

merged 2 commits into from
Dec 9, 2018

Conversation

smarlowucf
Copy link
Collaborator

Move collect vm info into distro to maintain consistency in architecture. Maintain init system check to maintain support for SysvInit.

This uses systemd so fits better on distro. If systemd does not
exist on the instance skip info.
@smarlowucf
Copy link
Collaborator Author

@rjschwei I know we agreed the other route. However, after thinking it over I think the code is easier to read and understand if we maintain the vm info method in distro class.

And we already have SysvInit support so no reason to drop it for now.

@rjschwei
Copy link
Collaborator

rjschwei commented Dec 7, 2018

@smarlowucf fair enough but then it should at least be in the distro base class and not in the sles implementation. I know we don't have rhel, debian etc. specific implementations at this point, but the systemd calls, while systemd version specific are distro agnostic. By putting the implementation into sles you imply that only sles has systemd-analyze and tha is just not correct. Please consider putting the implementation into the base class.

Only if init system isn't already set, attempt to retrieve and
set the value.
@smarlowucf
Copy link
Collaborator Author

Okay, moved them to base class and rebased the PR.

@rjschwei rjschwei merged commit e3349ac into master Dec 9, 2018
@smarlowucf smarlowucf deleted the systemd-cleanup branch December 10, 2018 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants